Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Google map3 58 #619

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Google map3 58 #619

wants to merge 4 commits into from

Conversation

k2-99
Copy link
Contributor

@k2-99 k2-99 commented Sep 9, 2024

Builds off of 51b0689, so I'd suggest just looking at the individual commits.

Apologies in advance for the styling getting picked up. I've since changed it to only format modifications on save. Let me know if the request is too time consuming to discern differences and I'll try to get one up without styling changes.

Why

  1. The current version is deprecated
  2. I want to use some of the newer features like vector/3d maps in the future :)
  3. Doing a large upgrade like this, touches a lot of places, so it helps me ramp up faster on how everything is fitting together from the UI perspective.

Major Changes:

Marker is deprecated

'AdvancedMarkerElement' is the replacement. Google's Migration Guide
Some key notes:

  1. Custom data isn't allowed in the constructor, so this is set after the marker is constructed now
  2. AdvancedMarker requires the use of MapId, googles way of setting map styles in the cloud. Note that with the use of Map ID, dynamically changing map styles locally is no longer supported for an existing map. You're forced to create a new map, which is then billed, so I removed this functionality (more details in map-styles.js).
  3. Icon can't be passed directly. Instead, it needs an HTML element. Also note that elements aren't cloned internally, so if you re-use an element in two markers, the map will 'move' it to the second assignment.
  4. Reduction in setters (e.g., marker.setMap, marker.setVisibility). Suggested alternative is to directly modify the property (marker.map = null is equivalent to marker.setMap(null))
  5. Needed to tweak the Z-indexes of the markers and the InfoWindows as by default with the new version, the markers were showing on-top of the info window.

New Constant

Given the MapId introduction above, this is now a constant that needs to be generated in the Google Map Cloud. See Google documentation on doing this. I currently have the constant in map-styles.js: GOOGLE_MAPS_DARK_STYLE_MAP_ID It's probably better suited for Constants.scala in the future though.

Known Bugs:

  1. Animation on Flight path seems slightly offset for some reason. I'm not sure why it's happening as the polyline remains using spherical as does the interpolate on the animation, both using the same to/from. Perhaps CSS related or some image artifact?
  2. Application of Map ColorScheme has a slight flicker at the start (shows light then swaps to dark right after init)

@patsonluk
Copy link
Owner

patsonluk commented Oct 14, 2024 •
edited
Loading

👋🏼 Many thanks for the change again. I have started another branch with only the code change (exclude space, semi-colon changes). https://github.com/patsonluk/airline/tree/google-map-3

Into the future, please make sure the PR has no space/styling changes, otherwise it's quite challenging to review the actual code changes and keep a relevant code history (i'm pretty sure you are aware of that already, since this is an older commit, doesn't hurt to bring that up again 😜 )

At this point, im reluctant to switch to V3 as u have already quite well documented with the styling issues. But just for reference in the future:

  1. The cost will likely be similar as we are using the dynmaic API already (which most of the time we disabled it due to exceeded quota since it's just too expensive...)
  2. Using Map ID itself is not too bad, to support 2 styles we will likely need to:
    i. Setting 2 Map ID, one with light and another with dark styling. From the look of the google community, they aren't going to allow changing style on the fly. which sucks
    ii. Because of i. we will likely need to force refresh to redraw everything :/
    iii. And the most importantly. It's just a pain to setup new map style. they don't even have any existing styling library to choose from, or import from legacy style json. It's a mess....im not going to rebuilding all the styling from ground up. Until they provide an easier mean to migrate, Im not going to switch.
  3. If google map really is a huge pain to use, we might need to consider other map api. Unfortunately, that could mean some major rewrite or removal of certain animations. We will see... (and cost is still the main concern, there's no free lunch... 🍽️ )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k2-99 @patsonluk